Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Benchmarks for paper #588

Merged
merged 20 commits into from
Mar 29, 2022
Merged

Benchmarks for paper #588

merged 20 commits into from
Mar 29, 2022

Conversation

olynch
Copy link
Member

@olynch olynch commented Dec 30, 2021

This contains updated benchmarks, along with code to generate figures for the paper. It also has a lot of miscellaneous optimizations; before this gets merged, I will look back and describe them.

Copy link
Member

@epatters epatters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are a few test failures. @olynch, can you fix them so we can get these performance improvements merged?

@olynch olynch force-pushed the struct-acsets-benchmarks branch 2 times, most recently from 750e6ec to 847a5d4 Compare January 26, 2022 21:44
Copy link
Member

@epatters epatters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Owen for this painstaking work on benchmarks!

Two important things should be resolved before merging:

  1. It looks like lots of new functions don't have unit tests. See inline comments for some of them.
  2. The use of @inlines seems a bit arbitrary, although I could be missing something. In my experience, inlines are only needed to ensure constant propagation for symbols. Can we be more consistent about the use of inlines and remove any extraneous ones?

src/graphs/BasicGraphs.jl Show resolved Hide resolved
src/categorical_algebra/CSetDataStructures.jl Show resolved Hide resolved
src/graphs/BasicGraphs.jl Show resolved Hide resolved
src/graphs/BasicGraphs.jl Outdated Show resolved Hide resolved
src/graphs/BasicGraphs.jl Show resolved Hide resolved
src/graphs/GraphGenerators.jl Show resolved Hide resolved
src/graphs/Searching.jl Show resolved Hide resolved
@epatters epatters merged commit 2180056 into master Mar 29, 2022
@epatters epatters deleted the struct-acsets-benchmarks branch March 29, 2022 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants